Wire MCPAuthzConfig references into workload controllers#4778
Closed
JAORMX wants to merge 2 commits into
Closed
Conversation
The existing AuthzConfigRef inline variant is Cedar-specific, with fields that only make sense for the Cedar authorizer. This adds a new MCPAuthzConfig CRD that accepts any registered authorizer backend configuration via a type discriminator and an opaque config blob. The MCPAuthzConfig CRD follows the established shared-config pattern (MCPOIDCConfig, MCPExternalAuthConfig) with status conditions, hash-based change detection, referencing workload tracking, and deletion protection. A new ConfigKey() method on the AuthorizerFactory interface enables the controller to reconstruct the full runtime config JSON from the CRD spec. The AuthzConfigRef field is added to MCPServer, MCPRemoteProxy, and VirtualMCPServer as a new option alongside the existing authzConfig field, enabling incremental migration with no breaking changes. Refs: #3157 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
MCPAuthzConfig resources created in PR 1 need to be consumed by the workload controllers (MCPServer, MCPRemoteProxy, VirtualMCPServer) so that shared authorization configs actually take effect at runtime. - Add CEL mutual exclusion for authzConfig/authzConfigRef on all specs - Add AuthzConfigHash to all workload status types for change detection - Add condition constants (AuthzConfigRefValidated) and reasons - Move BuildFullAuthzConfigJSON to controllerutil for shared use - Add controllerutil helpers: GetAuthzConfigForWorkload, ValidateAuthzConfigReady, GenerateAuthzVolumeConfigFromRef, EnsureAuthzConfigMapFromRef, AddAuthzConfigRefOptions - Add handleAuthzConfig to MCPServer, MCPRemoteProxy, VirtualMCPServer controllers following the OIDC config ref pattern - Add MCPAuthzConfig watches to all three workload controllers - Update runconfig builders to resolve AuthzConfigRef - Update vmcpconfig converter to extract Cedar policies from MCPAuthzConfig for VirtualMCPServer runtime config - Add SetAuthzConfigHash to VirtualMCPServer status manager Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
JAORMX
requested review from
ChrisJBurns,
jerm-dro,
jhrozek and
yrobla
as code owners
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## jaosorior/mcpauthzconfig-crd #4778 +/- ##
================================================================
- Coverage 68.85% 68.44% -0.41%
================================================================
Files 520 519 -1
Lines 55137 55199 +62
================================================================
- Hits 37964 37783 -181
- Misses 14242 14499 +257
+ Partials 2931 2917 -14 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Comment on lines
2338
to
+2356
| ) | ||
| } | ||
|
|
||
| // handleAuthzConfig validates and tracks the hash of the referenced MCPAuthzConfig. | ||
| // It updates the MCPServer status when the authorization configuration changes and sets | ||
| // the AuthzConfigRefValidated condition. | ||
| func (r *MCPServerReconciler) handleAuthzConfig(ctx context.Context, m *mcpv1alpha1.MCPServer) error { | ||
| ctxLogger := log.FromContext(ctx) | ||
|
|
||
| if m.Spec.AuthzConfigRef == nil { | ||
| // No MCPAuthzConfig referenced, clear any stored hash | ||
| if m.Status.AuthzConfigHash != "" { | ||
| m.Status.AuthzConfigHash = "" | ||
| if err := r.Status().Update(ctx, m); err != nil { | ||
| return fmt.Errorf("failed to clear MCPAuthzConfig hash from status: %w", err) | ||
| } | ||
| } | ||
| return nil | ||
| } |
Contributor
There was a problem hiding this comment.
🔴 When AuthzConfigRef is removed from an MCPServer or VirtualMCPServer spec, the ConditionAuthzConfigRefValidated condition is left stale in status instead of being cleared. This causes the condition to remain True indefinitely, misleading operators and any tooling that inspects status conditions about whether authorization is still active.
Extended reasoning...
Root cause — MCPServer
In MCPServer.handleAuthzConfig() (mcpserver_controller.go), the nil-ref branch clears AuthzConfigHash but never calls meta.RemoveStatusCondition:
if m.Spec.AuthzConfigRef == nil {
if m.Status.AuthzConfigHash != "" {
m.Status.AuthzConfigHash = ""
if err := r.Status().Update(ctx, m); err != nil { ... }
}
return nil // ← ConditionAuthzConfigRefValidated is never removed
}Root cause — VirtualMCPServer
In VirtualMCPServer.handleAuthzConfig() (virtualmcpserver_controller.go), the nil-ref branch has the same omission through the StatusManager abstraction:
if vmcp.Spec.IncomingAuth == nil || vmcp.Spec.IncomingAuth.AuthzConfigRef == nil {
if vmcp.Status.AuthzConfigHash != "" {
statusManager.SetAuthzConfigHash("")
}
return nil // ← no SetCondition/removal call for ConditionAuthzConfigRefValidated
}The reference implementation shows the correct pattern
MCPRemoteProxy.handleAuthzConfig() (introduced in the same PR) does it correctly:
if proxy.Spec.AuthzConfigRef == nil {
meta.RemoveStatusCondition(&proxy.Status.Conditions, mcpv1alpha1.ConditionAuthzConfigRefValidated)
if proxy.Status.AuthzConfigHash != "" { ... }
return nil
}Step-by-step proof (MCPServer)
- User creates an
MCPServerwithspec.authzConfigRef.name: my-authz. - Controller reconciles:
handleAuthzConfigfetches the config, validates it, setsConditionAuthzConfigRefValidated=Trueand stores the hash. - User removes
spec.authzConfigReffrom the MCPServer (sets to nil). - Controller reconciles:
handleAuthzConfigenters the nil branch, clearsAuthzConfigHash, and returns — no call tometa.RemoveStatusCondition. - Result:
kubectl get mcpserver <name> -o yamlstill showsConditionAuthzConfigRefValidated=Trueindefinitely, even though no authorization config is referenced.
The same sequence applies to VirtualMCPServer via the StatusManager path.
Why existing code doesn't prevent it
The StatusCollector for VirtualMCPServer is instantiated fresh each reconciliation with an empty conditions map. Conditions not explicitly set during a reconcile pass are left untouched in the live status (only conditions stored in s.conditions are applied in UpdateStatus). So the omission of a SetCondition call with removal intent means the live stale condition persists.
Fix
- MCPServer: Add
meta.RemoveStatusCondition(&m.Status.Conditions, mcpv1alpha1.ConditionAuthzConfigRefValidated)at the start of the nil branch, matching MCPRemoteProxy. - VirtualMCPServer: Call
statusManager.SetCondition(mcpv1alpha1.ConditionAuthzConfigRefValidated, "", "", "")or use the existing empty-Status marker trick inRemoveConditionsWithPrefixso the collector removes the condition inUpdateStatus.
jhrozek
previously approved these changes
Apr 13, 2026
jhrozek
left a comment
jhrozek
left a comment
Contributor
There was a problem hiding this comment.
Looks good -- solid pattern consistency with MCPOIDCConfig. Two minor nits inline.
| // set the type. Full support for other backends is a follow-up. | ||
| return &vmcpconfig.AuthzConfig{ | ||
| Type: authzConfig.Spec.Type, | ||
| }, nil |
Contributor
There was a problem hiding this comment.
nit: For non-Cedar types this returns an AuthzConfig with only Type set and no actual config. At runtime the vMCP would fail with an unclear error. Would it be better to fail fast here?
Suggested change
| }, nil | |
| return nil, fmt.Errorf("MCPAuthzConfig type %%q is not yet supported for VirtualMCPServer; only cedarv1 is supported", authzConfig.Spec.Type) |
Contributor
|
nit: // +kubebuilder:rbac:groups=toolhive.stacklok.dev,resources=mcpservers,verbs=get;list;watch
// +kubebuilder:rbac:groups=toolhive.stacklok.dev,resources=virtualmcpservers,verbs=get;list;watch
// +kubebuilder:rbac:groups=toolhive.stacklok.dev,resources=mcpremoteproxies,verbs=get;list;watch |
JAORMX
force-pushed
the
jaosorior/mcpauthzconfig-crd
branch
from
734e8db to
d571e89
Compare
JAORMX
force-pushed
the
jaosorior/mcpauthzconfig-crd
branch
from
d571e89 to
321c747
Compare
This was referenced May 29, 2026
ChrisJBurns
added a commit
that referenced
this pull request
Jun 11, 2026
No out-of-package caller exists for this helper today. PR #4778 will need it once workload controllers resolve AuthzConfigRef, and the cleaner home at that point will be cmd/thv-operator/pkg/controllerutil/ alongside BuildInlineCedarAuthzConfig. Until then keep the signature package-private so the contract can evolve without breaking external consumers. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
ChrisJBurns
added a commit
that referenced
this pull request
Jun 11, 2026
The staging note on AuthzConfigRef becomes inaccurate the moment the workload-controller wiring lands. A grep-able TODO(#4778) marker in the Go source ensures whoever lands #4778 finds the stale text without manually scanning three type files. controller-gen strips TODO lines from the rendered CRD description, so the user-facing schema stays clean. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
ChrisJBurns
added a commit
that referenced
this pull request
Jun 12, 2026
* Add MCPAuthzConfig CRD for backend-agnostic authz
Introduce a namespace-scoped MCPAuthzConfig CRD so authorization
policy can be defined once and shared across MCPServer,
MCPRemoteProxy, and VirtualMCPServer workloads, mirroring the
existing MCPOIDCConfig sharing pattern.
The spec carries a backend Type plus an opaque Config RawExtension.
A ConfigKey() method on AuthorizerFactory (cedar->"cedar",
http->"pdp") lets the controller reconstruct the full authorizer
config and validate it via the factory registry, keeping the
controller backend-agnostic; the backends are registered through
blank imports in the operator entrypoint.
The controller validates the spec, computes a config hash, manages a
finalizer, tracks referencing workloads, and blocks deletion while
referenced. AuthzConfigRef fields are added to the three workload
specs with CEL XValidation enforcing mutual exclusivity against the
existing inline authzConfig, matching the oidcConfig/oidcConfigRef
pattern.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Add mcpauthzconfigs to RBAC E2E golden fixtures
The chainsaw operator RBAC assertions compare the live
toolhive-operator-manager-role ClusterRole against a hardcoded
expected manifest. Adding the MCPAuthzConfig CRD introduced new
mcpauthzconfigs rules in the generated ClusterRole, making the
fixtures stale and failing E2E on all kind versions.
Add the mcpauthzconfigs, mcpauthzconfigs/finalizers, and
mcpauthzconfigs/status entries in the same alphabetical position the
generated role uses (after embeddingservers*, before
mcpexternalauthconfigs*) to both the multi-tenancy and single-tenancy
setup fixtures.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Guard reserved ConfigKey values and tighten MCPAuthzConfig.Validate
Register() now panics if a factory's ConfigKey() returns one of the
reserved envelope keys ("", "version", "type"). BuildFullAuthzConfigJSON
assembles its config with a Go map literal where those two keys are
fixed metadata; a factory returning a reserved key would silently
overwrite the envelope. Catching this at startup makes the
misconfiguration impossible to ship.
MCPAuthzConfig.Validate() now consults authorizers.IsRegistered so any
caller (CLI tooling, future webhook, defense-in-depth fallback) fails
closed on an unknown spec.type rather than deferring entirely to the
controller. The unit tests blank-import the cedar backend so the
existing "cedarv1" cases continue to pass.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* Canonicalize MCPAuthzConfig spec before hashing
The config hash is computed via ctrlutil.CalculateConfigHash, which walks
the spec struct including Spec.Config.Raw. RawExtension preserves the
user's raw bytes verbatim, so two semantically-equal configs that differ
only in whitespace or JSON key order produced different hashes and flipped
status on every noop kubectl-apply round-trip.
canonicalizeSpecForHash returns a copy of the spec whose Config.Raw has
been re-marshalled (Go's encoding/json sorts map keys), giving a stable
hash for the same logical config regardless of source formatting.
Malformed JSON is returned unchanged so Validate / validateAuthzConfig
remain the source of the user-facing error.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* Use MutateAndPatchStatus/Spec in MCPAuthzConfig controller
Status writes now flow through controllerutil.MutateAndPatchStatus and
finalizer writes through controllerutil.MutateAndPatchSpec, per the
operator rule (.claude/rules/operator.md). The previous r.Status().Update
calls sent full PUT bodies that would clobber conditions written by any
disjoint owner of Status.Conditions on this CRD; the previous r.Update
calls had no optimistic-lock guard around finalizer arrays.
Condition and field mutations have moved inside the helper closures so
the pre-mutate snapshot reflects the live state rather than already
containing the change — a MutateAndPatchStatus prerequisite. A small
helper, setValidTrueCondition, factors out the Valid=True transition so
the success path doesn't duplicate the metav1.Condition literal.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* Tighten MCPAuthzConfig reconcile flow
Collapses the previously-separate hash-change branch and steady-state
status update into a single MutateAndPatchStatus call on the success
path. Side effects of doing so:
- F4: ReferencingWorkloads / ReferenceCount now refresh in the same
reconcile that writes a new ConfigHash, so the print column doesn't
lag a workload event behind.
- F8: The success path explicitly removes the DeletionBlocked
condition. A user who cancels a deletion (e.g., by removing the
finalizer manually after observing the block) no longer carries a
stale DeletionBlocked=True forward across reconciles.
- F9: findReferencingWorkloads errors now return up to controller-
runtime instead of being logged and swallowed, so a transient
apiserver hiccup is retried with backoff rather than silently
writing an empty references list.
Two new tests exercise the F4 (one-reconcile property) and F8
(condition-clear) paths.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* Unexport buildFullAuthzConfigJSON
No out-of-package caller exists for this helper today. PR #4778 will
need it once workload controllers resolve AuthzConfigRef, and the
cleaner home at that point will be cmd/thv-operator/pkg/controllerutil/
alongside BuildInlineCedarAuthzConfig. Until then keep the signature
package-private so the contract can evolve without breaking external
consumers.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* Declare mcpservers RBAC marker on MCPAuthzConfig controller
findReferencingWorkloads lists MCPServer alongside VirtualMCPServer and
MCPRemoteProxy, but the kubebuilder marker only declared the latter
two. The clusterrole currently works because mcpserver_controller's
markers cover get;list;watch on mcpservers, so the rendered output is
unchanged here — but a future regeneration that reshuffles those
declarations could silently strip the permission the MCPAuthzConfig
controller actually depends on.
Verified the rendered clusterrole and chainsaw asserts are unchanged
after task operator-manifests.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* Test condition preservation and last-ref finalizer transition
Two new tests for MCPAuthzConfig reconciler coverage:
- FinalizerRemovedAfterLastRefDropped: exercises the N→0 transition
in handleDeletion. The existing static-state cases cover "blocked"
and "no refs" endpoints but not the flip between them, which is the
user-observed behaviour for policy rotation.
- PreservesForeignConditions: the canary for the merge-patch
conditions-array semantic. JSON merge-patch on CRDs replaces the
conditions array wholesale (the +listType=map marker only matters to
strategic-merge-patch), so a regression that re-introduces
r.Status().Update on this resource — or that pre-mutates Status
outside a MutateAndPatchStatus closure — would erase any concurrently-
set foreign condition. This test fails in that scenario.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* Add envtest for authzConfig/authzConfigRef mutual exclusion
CEL XValidation rules run at API admission, which unit tests with the
fake client cannot exercise. The three workload specs (MCPServer,
MCPRemoteProxy, VirtualMCPServer.IncomingAuth) each declare the same
mutex rule between the legacy inline authzConfig and the new
authzConfigRef — without envtest coverage the rules could silently
regress on a CRD regeneration.
For each spec, the new tests apply only-inline, only-ref, and both-set
CRs and assert (c) is rejected with the expected message. They follow
the existing pattern in cmd/thv-operator/test-integration/.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* Reframe AuthzConfigRef as part 1 of 2 in godocs
The new AuthzConfigRef field on MCPServer, MCPRemoteProxy, and
VirtualMCPServer.IncomingAuth is wired into the MCPAuthzConfig
controller's reference tracking (status.referenceCount, deletion
protection) but no workload controller actually resolves the ref into a
runtime authz config in this PR. That wiring lands in a follow-up.
The previous godocs marked the inline AuthzConfig field "Deprecated:
Use AuthzConfigRef" — which pointed users at a non-functional field
and risked workloads running with no authorization enforced. Drop the
premature Deprecated annotation and add an explicit NOTE on
AuthzConfigRef so adopters know to stick with the inline form until
the consumer-side wiring lands.
The CEL mutex rule and the controller's reference tracking are
unchanged; only the descriptive godocs move.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* Regenerate CRD manifests and reference docs
Picks up the F1 godoc edits: the CRD YAML descriptions and the
generated CRD API reference (docs/operator/crd-api.md) now match the
authoritative godoc on AuthzConfigRef / AuthzConfig (no premature
Deprecated annotation; explicit "consumed in a follow-up PR" note on
AuthzConfigRef).
Generated by task operator-generate + task operator-manifests +
task crdref-gen.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* Use compound (kind, name) listMapKey on ReferencingWorkloads
The original review of this PR flagged that +listMapKey=name alone is
insufficient: two workloads of different kinds that share a name (an
MCPServer "foo" and a VirtualMCPServer "foo") would collide under
merge-patch semantics, with the second-applied entry silently
overwriting the first. Adding +listMapKey=kind makes the map key the
(kind, name) pair so cross-kind name reuse stays distinct.
Limited to MCPAuthzConfig here. The two sibling controllers
(MCPOIDCConfig, MCPExternalAuthConfig) share the same WorkloadReference
type and have the same listMapKey=name asymmetry — filed as a parity
follow-up rather than expanding this PR's scope.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* Refresh validateAuthzConfig doc comment
The prior comment said backend validation lived in the controller "because
the API types package must not import the authorizer backends" — but the
types package now imports pkg/authz/authorizers to call IsRegistered in
Validate(). Restate the real reason: type-level Validate handles structural
+ registration checks; backend ValidateConfig requires the full
reconstructed JSON envelope that only the controller builds.
Drive-by: gitignore .pr-review/ so PR-review skill scratch dirs don't
pollute git status in worktrees.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* Thread factory through buildFullAuthzConfigJSON, drop redundant lookups
buildFullAuthzConfigJSON now returns the resolved AuthorizerFactory
alongside the envelope JSON, so validateAuthzConfig can dispatch
ValidateConfig directly without a second GetFactory call and without
re-Unmarshalling the JSON it just built.
Before: Validate() ran IsRegistered, buildFullAuthzConfigJSON ran
GetFactory, validateAuthzConfig re-Unmarshalled the envelope and ran
GetFactory again — three near-identical lookups for the same key on
every reconcile, with three slightly different error messages.
The authzConfigEnvelope struct is removed; nothing else consumed it.
Tests updated to assert the returned factory's ConfigKey matches the
expected nested envelope key, locking in the bidirectional contract.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* Drop marshalJSONString in favor of strconv.Quote
json.Marshal of a Go string cannot fail at runtime — encoding/json's
stringEncoder has no error path for valid Go strings. The helper existed
to placate errcheck rather than to catch a real failure mode, adding two
unreachable error branches per call site. strconv.Quote produces the
same JSON-encoded bytes with no error-checking overhead, and the call
sites become readable single expressions.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* Demote hash-change log to DEBUG
Silent-success per .claude/rules/go-style.md: routine state transitions
log at DEBUG, INFO is reserved for long-running operations. A noisy
INFO on every kubectl-apply that bumps a hash is exactly the case the
rule was written to prevent.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* Mark AuthzConfigRef staging NOTE with TODO(#4778)
The staging note on AuthzConfigRef becomes inaccurate the moment the
workload-controller wiring lands. A grep-able TODO(#4778) marker
in the Go source ensures whoever lands #4778 finds the stale text
without manually scanning three type files. controller-gen strips
TODO lines from the rendered CRD description, so the user-facing
schema stays clean.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* Lock ConfigKey()/struct tag contract per backend
Factory.ConfigKey() returns a string ("cedar" / "pdp") that must match
the json tag on the backend's Config.Options field — otherwise the
controller will emit envelope JSON the backend's own Unmarshal cannot
parse. A new TestFactory_ConfigKeyMatchesStructTag per backend builds
an envelope around ConfigKey() and asserts it deserialises with
Options populated. A future rename of either string in isolation will
fail the test.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* Tighten test docstrings, fixtures, and assertions
PreservesForeignConditions: docstring reframed to describe the property
it actually guards (snapshot-and-diff doesn't erase foreign conditions
during patch construction) and to be explicit about what it does NOT
catch (r.Status().Update under the fake client — requires a
WithInterceptorFuncs-backed concurrent-writer scenario).
HashAndRefsLandInOneReconcile: the MCPServer fixture now sets the
required Image field so the test remains portable to envtest. Added an
ObservedGeneration assertion so all four fields written in the single
MutateAndPatchStatus closure are pinned (previously hash, references,
referencingWorkloads.name were locked but ObservedGeneration was not).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* Update cmd/thv-operator/controllers/mcpauthzconfig_controller.go
Co-authored-by: Jakub Hrozek <jakub.hrozek@posteo.se>
* Remove unused strconv import
Cascade fix from the web-UI edit that replaced strconv.Quote with
json.Marshal: the import was left in place, breaking compilation and
every downstream CI job (lint, tests, govulncheck x2, helm lint, e2e
lifecycle x3).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Chris Burns <29541485+ChrisJBurns@users.noreply.github.com>
Co-authored-by: Jakub Hrozek <jakub.hrozek@posteo.se>
ChrisJBurns
requested review from
blkt,
rdimitrov,
reyortiz3 and
tgrunnagle
as code owners
10 tasks
ChrisJBurns
added a commit
that referenced
this pull request
Jun 18, 2026
* Add MCPAuthzConfig ref-resolution foundation Foundation (Stage 1 of #4778's re-derivation) for making spec.authzConfigRef enforce at runtime. Purely additive: no workload controller is wired yet and the inline spec.authzConfig path is untouched. - Add controllerutil ref-helpers in authz_ref.go: GetAuthzConfigForWorkload, ValidateAuthzConfigReady, AddAuthzConfigRefOptions (resolves a referenced MCPAuthzConfig into a runner authz.Config for any registered backend — cedarv1 and httpv1 — via the authorizers factory), and the EnsureAuthzConfigMapFromRef / GenerateAuthzVolumeConfigFromRef materialization helpers mirroring the inline ones. - Export and move BuildFullAuthzConfigJSON from the MCPAuthzConfig controller into controllerutil so both the config controller and the workload controllers share it without an import cycle; the controller now calls the shared helper. - Add the AuthzConfigHash status field to MCPServer, MCPRemoteProxy, and VirtualMCPServer, plus ConditionAuthzConfigRefValidated and reason constants, mirroring the OIDCConfigRef equivalents. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Regenerate CRD manifests and docs for AuthzConfigHash Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Address review feedback on authz ref helpers - Guard EnsureAuthzConfigMapFromRef with ValidateAuthzConfigReady so the exported helper never materializes a ConfigMap from a config flagged invalid (defense in depth; no extra I/O — the object is already fetched). (F2) - Clarify the authzRefConfigMapName doc comment: the ConfigMap *name* is distinct from the inline path, while the *volume* name and mount path are deliberately shared (inline and ref are mutually exclusive via CRD XValidation, so a workload mounts at most one authz volume). (F3) - Restore the nil data/factory assertions on BuildFullAuthzConfigJSON's error branch. (F5) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
10 tasks
Collaborator
|
Closing as its being superseeded |
ChrisJBurns
added a commit
that referenced
this pull request
Jun 19, 2026
The AuthzConfigRef field docs on MCPServer and MCPRemoteProxy still carried the TODO(#4778) + NOTE saying the ref is reference-tracked but does NOT apply authorization. Both controllers now resolve the ref into runtime authz, so the note states the opposite of the behavior and would mislead anyone reading the generated CRD reference. Remove it and regenerate manifests + API docs. Addresses review feedback on #5563. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Type of change
Test plan
task test)task lint-fix)Changes
api/v1alpha1/mcpserver_types.goapi/v1alpha1/mcpremoteproxy_types.goapi/v1alpha1/virtualmcpserver_types.gopkg/controllerutil/authz.gocontrollers/mcpauthzconfig_controller.gocontrollers/mcpserver_controller.gocontrollers/mcpserver_runconfig.gocontrollers/mcpremoteproxy_controller.gocontrollers/mcpremoteproxy_runconfig.gocontrollers/virtualmcpserver_controller.gopkg/virtualmcpserverstatus/types.gopkg/virtualmcpserverstatus/collector.gopkg/vmcpconfig/converter.goSpecial notes for reviewers
BuildFullAuthzConfigJSONwas moved from the MCPAuthzConfig controller tocontrollerutilso both the config controller and workload controllers can use it without circular imports.Generated with Claude Code